Skip to content

stty: Changing shell command to add recognizing a TTY for stty tests #9336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

stty: Changing shell command to add recognizing a TTY for stty tests #9336

wants to merge 18 commits into from

Conversation

@ChrisDryden
Copy link
Contributor

@ChrisDryden ChrisDryden commented Nov 19, 2025 •
edited
Loading

Been looking into examples of how other repositories using Github actions are able to test things that require the tests recognizing that a TTY session exists. This example comes from: https://github.com/gfx/example-github-actions-with-tty and is used in a bunch of other open source projects such as: https://cocalc.com/github/Ardupilot/ardupilot/blob/master/.github/workflows/test_environment.yml

The integration test suite ran and showed that now the tests are running for stty! The tests are not passing because there is still some things to fix in STTY itself and those are being worked on in other PR's

@ChrisDryden ChrisDryden changed the title Changing shell command to add recognizing a TTY for stty tests stty: Changing shell command to add recognizing a TTY for stty tests Nov 19, 2025
@ChrisDryden ChrisDryden mentioned this pull request Nov 19, 2025
Open
@github-actions
Copy link

github-actions bot commented Nov 20, 2025

GNU testsuite comparison:

GNU test failed: tests/stty/bad-speed. tests/stty/bad-speed is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/stty/stty. tests/stty/stty is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/stty/stty-invalid. tests/stty/stty-invalid is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/stty/stty-pairs. tests/stty/stty-pairs is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/stty/stty-row-col. tests/stty/stty-row-col is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 20, 2025

@sylvestre The "GNU testsuite comparison" is showing now that the STTY tests are not being skipped and are running! From what I can gather about the other two tests, it appears to be just flaky tests unrelated to this change. Would you be open to taking this approach to change these five tests from "Skipped" to failed?

I have other PR's open that address the failures themselves, should have these tests passing in the CI soon!

This was referenced Nov 23, 2025
Merged
Open
@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 24, 2025

Added additional comments describing the change and re-based. I'm expecting it to still fail the GNU test check since it is not able to recognize a test that moved from skipped to fail.

@github-actions
Copy link

github-actions bot commented Nov 24, 2025

GNU testsuite comparison:

GNU test failed: tests/stty/bad-speed. tests/stty/bad-speed is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 24, 2025

Darn compared to the other runs for some reason this time it only recognized the pty for one out of the five tests. Still some work to go

@ChrisDryden ChrisDryden marked this pull request as draft November 24, 2025 16:43
@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 24, 2025

Changing it to this new format appears to consistently run the STTY tests

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 24, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #9336 will not alter performance

Comparing ChrisDryden:patch-1 (48ffbb7) with main (821c674)

Summary

✅ 126 untouched
⏩ 6 skipped1

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@ChrisDryden ChrisDryden marked this pull request as ready for review November 24, 2025 21:07
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

@ChrisDryden ChrisDryden marked this pull request as draft November 24, 2025 22:01
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

1 similar comment
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@github-actions
Copy link

github-actions bot commented Nov 26, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 26, 2025

Two of the stty tests are now passing in the main branch!

@sylvestre
Copy link
Contributor

sylvestre commented Nov 26, 2025

Would you be open to taking this approach to change these five tests from "Skipped" to failed?

of course, failed is better than skipped!

@github-actions
Copy link

github-actions bot commented Nov 29, 2025

GNU testsuite comparison:

GNU test failed: tests/stty/bad-speed. tests/stty/bad-speed is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/stty/stty. tests/stty/stty is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/stty/stty-row-col. tests/stty/stty-row-col is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/stty/stty-invalid is now passing!
Congrats! The gnu test tests/stty/stty-pairs is now passing!

@ChrisDryden ChrisDryden mentioned this pull request Nov 29, 2025
Merged
@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 29, 2025

Right now the bad-speed test is timing out making which adds too much time to the test. Once all of the stty fixes are merged this will change to seconds and should be good to merge.

@github-actions
Copy link

github-actions bot commented Nov 29, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/stty/stty-invalid is now passing!
Congrats! The gnu test tests/stty/stty-pairs is now passing!
Note: The gnu test tests/stty/bad-speed was skipped on 'main' but is now failing.
Note: The gnu test tests/stty/stty was skipped on 'main' but is now failing.
Note: The gnu test tests/stty/stty-row-col was skipped on 'main' but is now failing.

@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 29, 2025

The other change to track the skipped to failing is working now!

@ChrisDryden
Copy link
Contributor Author

ChrisDryden commented Nov 29, 2025 •
edited
Loading

I would still say not ready for merge since the bad-speed test is timing out, but once: #9490 #9516 and #9517 are in all should be passing and not be timing out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChrisDryden @sylvestre